Skip to content

fix(query): skip inactive rows in vectorized LIKE - #20211

Open
dantengsky wants to merge 1 commit into
databendlabs:mainfrom
dantengsky:fix/like-validity-short-circuit
Open

fix(query): skip inactive rows in vectorized LIKE#20211
dantengsky wants to merge 1 commit into
databendlabs:mainfrom
dantengsky:fix/like-validity-short-circuit

Conversation

@dantengsky

@dantengsky dantengsky commented Jul 28, 2026

Copy link
Copy Markdown
Member

I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/

Summary

AND evaluation passes rows selected by earlier predicates through EvalContext.validity. The
custom vectorized string LIKE implementation ignored this bitmap and still ran the matcher for
every row in the input column. This is especially expensive when FUSE Parquet Prewhere combines a
selective predicate with one or more LIKE predicates.

For example:

SELECT event_time, payload
FROM benchmark_events
WHERE worker = 'worker-7'
  AND payload NOT LIKE '%absent_token%'
  AND category NOT LIKE '%controller.module%'
ORDER BY event_time DESC
LIMIT 50;

If worker = 'worker-7' selects 1/64 of the rows, the two LIKE predicates previously still
processed the complete columns. They now process only rows whose validity bit is true.

The change covers column LIKE scalar, scalar LIKE column, and column LIKE column. It keeps the
existing full-column path when validity is absent or all true, and retains the specialized
%literal% substring search path.

On an S3-backed synthetic FUSE table with 10 million rows and approximately 2 KiB strings, the
query above improved from a 13.50 s median to 1.60-1.74 s with default settings. Both plans read the
same 10 million rows; LIKE evaluations dropped from 10 million to 156,250 per predicate. Aggregate
results and the Top-50 result fingerprint were identical before and after the change.

Tests

  • Unit Test
  • Logic Test
  • Benchmark Test
  • No Test - Explain why

Added tests for:

  • sparse validity across all three vector argument shapes;
  • matcher call counts for inactive rows;
  • all-true validity and the specialized substring path;
  • 128 property-test cases comparing sparse and full evaluation for active rows.

Type of change

  • Bug Fix (non-breaking change which fixes an issue)
  • New Feature (non-breaking change which adds functionality)
  • Breaking Change (fix or feature that could cause existing functionality not to work as expected)
  • Documentation Update
  • Refactoring
  • Performance Improvement
  • Other (please describe):

This change is Reviewable

@sundy-li sundy-li left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The validity short-circuit logic looks sound: inactive rows produce false without invoking pattern conversion/matching, active-row results remain equivalent to dense evaluation, and the specialized SurroundByPercent path is preserved. Focused tests pass on the PR head.\n\nThis PR now conflicts with current main after #20207 merged, in the shared comparison test module. Please rebase/update the branch and retain both the escaped-LIKE/VARIANT coverage from #20207 and this PR's sparse-validity coverage. I resolved that conflict locally and all 13 comparison tests passed, so it appears mechanical, but I am withholding approval until the final rebased head is available for verification.

@sundy-li

Copy link
Copy Markdown
Member

One additional validity-contract concern: EvalContext.validity is also propagated during nullable and conditional partial evaluation, not only by and_filters/or_filters. The new code changes inactive rows' inner Boolean payload to false. That payload should be unobservable under normal nullable semantics, but the current direct vectorizer tests only prove sparse = validity && dense in a filter-style context.

Please add an evaluator-level regression with nullable input and a non-filter wrapper/conditional, for example combinations using if, is_true, coalesce and not, to confirm SQL-visible values and output validity remain unchanged. assume_not_null/remove_nullable deserves explicit consideration: it can expose the inner payload of NULL rows, so a partially evaluated assume_not_null(nullable_string LIKE '%') can observe false after this patch where the previous matcher might have produced true from the nullable column's default inner string. If that is intentionally outside the semantic contract, documenting/testing the supported contract would make the optimization boundary clear.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-bugfix this PR patches a bug in codebase

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants